feat(sqllab): Add a configuration option to disable data preview #19104
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This setting is per-database in the "extra" JSON data.
… sure the code is compatible with master
…kboxes and tooltips but there are now 8
…kboxes and tooltips but there are now 8
…n the PR. The largest change was reversing the flag from 'allows_preview_data' to 'disable_preview_data'. This required changes to a lot of the files, as well as I needed to reverse the expected boolean values on some of the tests
…ring is passed into the parameter instead of a boolean value, it will return False
|
Hi @villebro! Would you be able to review this PR when you get the chance? It has all of the modifications you commented on in the original PR. |
…eCanada/superset into update-configure-data-preview
Codecov Report
@@ Coverage Diff @@
## master #19104 +/- ##
=======================================
Coverage 66.73% 66.74%
=======================================
Files 1668 1668
Lines 64271 64279 +8
Branches 6496 6496
=======================================
+ Hits 42894 42902 +8
Misses 19695 19695
Partials 1682 1682
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
cccs-Dustin
changed the title
villebro
changed the title
villebro
reviewed
Mar 15, 2022
superset-frontend/src/views/CRUD/data/database/DatabaseModal/ExtraOptions.tsx
Outdated
Show resolved
Hide resolved
superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx
Outdated
Show resolved
Hide resolved
cccs-Dustin
changed the title
…lag to be named 'disable_data_preview'
villebro
approved these changes
Mar 17, 2022
There was a problem hiding this comment.
Thanks for this awesome improvement @akedrou and @cccs-Dustin , LGTM!
8 tasks
villebro
pushed a commit
that referenced
this pull request
Apr 3, 2022
) (cherry picked from commit 02ef9ca)
mistercrunch
added
🍒 1.5.0
🍒 1.5.1
🍒 1.5.2
🍒 1.5.3
🏷️ bot
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
SUMMARY
This PR is a rebase of the original #14768 PR created by @akedrou. The original modifications made to the code were kept intact, however, to ensure that the feature works on master, I had to make some minor modifications to specific files. I also made some edits/additions based on the feedback that was given on the original PR.
Original PR's Summary:
This PR adds a new per-database configuration option to disable data preview queries in SQL Lab. This setting is exposed in the database API as disable_preview_data. I decided to add the feature to the database API and pass the database as a parameter to addTable rather than adding a database setting into a table API response (the proposed solution in #14726). As a result, the data preview query was moved up to addTable as well. To my eye that looks cleaner since I don't think that previewing data should be a part of "table metadata" anyway.
There is no UI change, though the UX changes when data preview is disabled to not show the data preview pane (or results) when getting table metadata.
NOTE: The original PR mentions that there is no UI change, this is no longer the case as there is now a new checkbox in the "Advanced>SQL Lab" section of a database settings page.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A
TESTING INSTRUCTIONS
To test that the SQL Lab data preview is disabled when the checkbox is selected
- Alternately, trigger metadata to load from the editor textbox itself.
To test that the SQL Lab data preview works when the checkbox is not selected
- Alternately, trigger metadata to load from the editor textbox itself.
You can also run the following tests to make sure they still pass after the code modifications:
npm test ./src/components/DatabaseSelector/DatabaseSelector.test.tsxnpm test ./src/SqlLab/actions/sqlLab.test.jsnpm test ./src/SqlLab/components/SqlEditor/SqlEditor.test.jsxnpm test ./src/SqlLab/components/SqlEditorLeftBar/SqlEditorLeftBar.test.jsxtox -e py38 -- tests/integration_tests/core_tests.pytox -e py38 -- tests/integration_tests/databases/api_tests.pyADDITIONAL INFORMATION